Store operation content once and enforce per-plan history retention - #35
Merged
Conversation
…tion `operations` and `operation_files` were the only unbounded tables, and every file body was written three times: once in `operations.event` (the envelope, whose payload is the transaction), again in `operations.transaction`, and again in `operation_files.payload`. Migration 012 drops the latter two, making the envelope the single home for content and leaving `operation_files` as the per-path index into it. GET /v1/operations is byte-identical either way, since jsonb canonicalizes a value the same wherever it is stored. Retention is now enforced against PLAN_LIMITS[plan].historyRetentionDays. Pruning `operations` used to be unsafe because cursor reconnect cannot tell a truncated list from "caught up", so the sweep deletes strictly a prefix of each workspace's sequence and records how far it reached in workspaces.operations_pruned_through. A cursor below that watermark is answered with an explicit cursor-too-old resync status instead of a short page, and with 410 Gone for daemons that predate the status, so an older client cannot read it as success. The daemon adopts the watermark, logs the gap, and surfaces it in `status().service`. The sweep runs on a service-side interval with its own privileged connection, because the request-serving role deliberately cannot delete operations; `pnpm service:prune` remains the manual tool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Conflicts were three, all mechanical unions: - store.ts imports and the migration list: main added the self-serve workspace cap and 012_rate_limits.sql, this branch added PLAN_LIMITS and its own migration. Both are kept, and this branch's migration is renumbered to 013 since main took 012. - store.integration.test.ts imports: both sides added some. - BUILD_INSTRUCTIONS.md: main documented the free-tier abuse guards where this branch replaced the "retention is declared but not enforced" entry. main also moved the API onto a function platform, which has no persistent process to run the retention interval. Noted in serverless.ts alongside the other things that do not survive that move, and in the README: until a platform cron exists, that deployment needs `pnpm service:prune` driven externally. Reads stay correct there regardless, since nothing being deleted means no cursor is ever refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Coordination service, daemon, protocol, and docs: migration 012 drops
operations.transactionandoperation_files.payloadso a transaction's file bodies live only inoperations.event(they were stored three times), leavingoperation_filesas the per-path index into it.PgStore.pruneOperationsByRetention()now deletes each workspace's operations outsidePLAN_LIMITS[plan].historyRetentionDays— strictly a prefix of the sequence — and records the highest deletedserver_sequenceinworkspaces.operations_pruned_through, soGET /v1/operationscan answer a cursor below that watermark with an explicitcursor-too-oldresync status (or410 Gonefor daemons predating it, gated on a newprotocolVersionparameter) instead of a truncated page that reads as "caught up"; the daemon adopts the watermark, records it, and surfaces the gap instatus().service. The sweep runs on a service-side interval with its own privileged connection (CROSSCODE_RETENTION_DATABASE_URL), since the request-serving role deliberately cannot delete operations, andpnpm service:prunestays the manual tool.Why
operations/operation_fileswere the only unbounded tables and the real hosting cost: every byte ofafterContentwas written three times, and the declared per-plan retention was never enforced because a naive age-basedDELETEwould hand a long-offline replica a short list it would silently mistake for being up to date.Testing
GET /v1/operationsis proven byte-identical before and after by comparing the serialized response against the same transaction read back throughSELECT $1::jsonb(what the dropped column stored), and a sentinel census asserts the content appears exactly once acrossoperationsand zero times inoperation_files. The silent-truncation case has dedicated tests at every layer: the store (asserting the naive query returns the empty list a replica would misread as "caught up"), the HTTP boundary, the protocol schemas, the daemon's resync path, and end-to-end over a real service, daemon, and Postgres.pnpm buildpassespnpm testpasses (305 passed)pnpm test:postgrespasses (24 passed across 7 files, none skipped)Security / trust-boundary impact
The append-only guarantee on the request path is preserved:
assertRuntimePrivilegesstill refuses a runtime role that can deleteoperations, and retention deletes only through a separate connection configured withCROSSCODE_RETENTION_DATABASE_URL, so no request-handling code path can reach a connection able to erase history (documented in docs/security.md). A resync only ever discards proposals a replica never downloaded — Git remains the source of truth, so no committed or working-tree work is at risk.Related issues
None.
🤖 Generated with Claude Code